Reject on /api/eth-rpc in direct_api_call#389
Conversation
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
Warning Review limit reached
More reviews will be available in 55 minutes and 59 seconds. Learn how PR review limits work. Your organization has run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (2)
WalkthroughThe PR implements an explicit corrective rejection for requests to the legacy Blockscout JSON-RPC endpoint ChangesLegacy JSON-RPC path validation and rejection
Possibly Related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Normalize endpoint_path (strip query string, whitespace, trailing slash, case) before comparing to the legacy path, so the corrective /json-rpc message wins over the generic query-param error and surrounding-whitespace variants are caught too. Add tests for the query-string and whitespace variants, plus negative tests confirming a lookalike (/api/eth-rpc-foo) and the supported /json-rpc path are not rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ess count
- Strip surrounding whitespace from endpoint_path once up front so supported
paths are normalized the same way as the legacy-path guard (previously only
the rejection comparison trimmed whitespace, leaving " /json-rpc " to fail
opaquely upstream).
- Drop the now-redundant .strip() from the guard comparison; the remaining
.rstrip("/") still handles the slash-before-query case (/api/eth-rpc/?id=1).
- Remove the report_progress.await_count assertions from the rejection tests;
assert_not_awaited() already proves the rejection is pre-network, and the
count coupled the tests to the internal progress sequence.
- Add a test asserting whitespace on a supported path is stripped before the
network call.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Strip whitespace from the pre-query segment so "/api/eth-rpc ?id=1" folds onto the same rejection instead of falling through to the generic query-param error. Add two rejection tests: trailing-slash + query string, and whitespace before the query string. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@tests/tools/direct_api/test_direct_api_call_validation.py`:
- Around line 155-170: The test currently only asserts that
make_blockscout_request was awaited once; update
test_direct_api_call_allows_non_legacy_lookalike_path to assert the exact helper
call arguments by checking mock_get.assert_awaited_once_with(...) so the call
from direct_api_call(chain_id="1", endpoint_path="/api/eth-rpc-foo",
ctx=mock_ctx) passes the exact api_path and params contract to
make_blockscout_request (use the api_path value derived from endpoint_path and
the params derived from chain_id/ctx used by direct_api_call); do the same style
of assertion for the other test(s) mentioned (around lines 173-190) to lock down
normalization/regression scenarios.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d3d84af3-ab4e-4879-84ce-72dffb081689
📒 Files selected for processing (7)
AGENTS.mdSPEC.mdblockscout_mcp_server/__init__.pyblockscout_mcp_server/tools/direct_api/direct_api_call.pypyproject.tomlserver.jsontests/tools/direct_api/test_direct_api_call_validation.py
…ests Tighten the three pass-through tests from assert_awaited_once() to assert_awaited_once_with(...) so they lock down the exact api_path/params (and json_body for POST) contract passed to the network helper. This guards the look-alike path against silent rewrites and pins normalization behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Condense the two-sentence description to a single clause that keeps the behavioral contract (pre-network rejection, error names /json-rpc, never silently rewritten) and drops the PR-rationale editorializing, matching the density of the surrounding Implementation paragraph. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
main already shipped 0.16.0.dev12 via #389 after this branch diverged, so re-bump to dev13 to keep the dev version monotonic on merge. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 participant
Closes #386.
Motivation
direct_api_callmoved from the historical/api/eth-rpcpath to/json-rpc. A caller (human or agent) that still requests the old path would previously hit one of two poor outcomes: a silent rewrite (opaque, and hides the convention change) or an opaque upstream gateway error — and either way, the request consumed an authenticated PRO API credit on something known to fail./api/eth-rpcis still requested, the tool fails fast with a corrective error that names the supported/json-rpcpath, before any network call and before any credit is spent.SPEC.md. It is deliberately not added to the tool's docstring/description: that text loads into every agent's context on every call, so documenting a rarely-needed legacy-path mistake there would pollute context for no benefit.Description
direct_api_call, added the rejection inside the existing path-validation block, immediately after whitespace/trailing-slash normalization and before the?-in-path check so the corrective legacy-path message wins over the generic query-param error.endpoint_pathis stripped of surrounding whitespace once up front (the same normalization then applies to every path, not just the legacy one), and a single normalized comparison —endpoint_path.split("?", 1)[0].rstrip("/").lower() == "/api/eth-rpc"— folds the bare path, the trailing-slash variant (/api/eth-rpc/), capitalization variants (/API/ETH-RPC), and a trailing query string (/api/eth-rpc?id=1) onto one rejection. On a match it raisesValueError("The legacy JSON-RPC path '/api/eth-rpc' is no longer supported. Retry with endpoint_path='/json-rpc'.")— it never silently rewrites the path; transparency is the point. Placing the guard before the network call (make_blockscout_request/make_blockscout_post_request) is what guarantees no credit is spent. Added a focused unit-test moduletests/tools/direct_api/test_direct_api_call_validation.py(a new file, sincetest_direct_api_call.pyis already at the 500-LOC cap) with nine tests: six rejection cases (GET bare path, trailing-slash, POST, case-insensitive, trailing query string, surrounding whitespace), each asserting the network helper was not awaited; and three pass-through cases (a non-legacy look-alike/api/eth-rpc-foo, the supported/json-rpcpath, and a whitespace-padded supported path whoseapi_pathis normalized before the network call).direct_api_callImplementation paragraph inSPEC.mdwith two sentences describing the legacy-path rejection as part of the same pre-network validation, and added the new test module to theAGENTS.mdproject-structure tree. No agent-facing context surface was touched: the tool docstring,API.md,gpt/*, andTESTING.mdare unchanged (they already reference/json-rpconly).API.mdneeds no change because the operation's contract (path, method, parameters, response shape) is unchanged — only the runtime behavior for one specific input value is refined.0.16.0.dev11to0.16.0.dev12in the three synced locations (pyproject.toml,blockscout_mcp_server/__init__.py,server.json).mcpb/manifest.jsonis intentionally left unchanged (rule 135): no tool was added or removed and the tool description is unchanged.Testing
uv run pytest -m "not integration"— green, including the nine new validation tests.uv run python scripts/run_integration_tests.py); the existingtest_direct_api_call_real.pyalready exercises the supported/json-rpcpath. No new integration test was added — the new behavior is pure offline input validation with no network call.uv run ruff check .anduv run ruff format --check .both clean.Summary by CodeRabbit
New Features
direct_api_calltool now validates endpoint paths before processing requests, rejecting legacy paths and providing guidance to use the correct endpoint.Chores